Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vpc20 service profile support #1286

Merged
merged 1 commit into from
Jul 31, 2024
Merged

Conversation

annakhm
Copy link
Collaborator

@annakhm annakhm commented Jul 28, 2024

Extended tests will be added as follow up

@annakhm annakhm force-pushed the vpc20-service-profile2 branch 3 times, most recently from 29e0755 to f1e1ec4 Compare July 29, 2024 22:14
SdkFieldName: "IsDefault",
},
},
"mac_discovery_profile": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Various profiles are policy paths? If so, they can be validated with validatePolicyPath()

var err error
parents := getVpcParentsFromContext(sessionContext)
client := clientLayer.NewVpcServiceProfilesClient(connector)
_, err = client.Get(parents[0], parents[1], id)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is simpler to take org/project/VPC from context than loading these into parents array and using from there

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the generator this way is a cleaner and more readable option I think. The only difference is function name, rather than rewriting the whole block.

testResourceName := "nsxt_vpc_service_profile.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccOnlyMultitenancy(t) },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I'm not sure if that should be tested with multitenancy or with VPC env. For now definitely with VPC as it runs on v9.0 (these tests will fail with v4.1.2).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be part of VPC test suite later on too

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So let's replace testAccOnlyMultitenancy(t) with testAccOnlyVPC(t)

@annakhm annakhm force-pushed the vpc20-service-profile2 branch from f1e1ec4 to b16de35 Compare July 30, 2024 18:30
@annakhm annakhm mentioned this pull request Jul 30, 2024
Extended tests will be added as follow up

Signed-off-by: Anna Khmelnitsky <[email protected]>
@annakhm annakhm force-pushed the vpc20-service-profile2 branch from b16de35 to 8b44c1b Compare July 31, 2024 15:53
@annakhm annakhm merged commit 2b392c7 into vpc20-feature-branch Jul 31, 2024
4 checks passed
@annakhm annakhm deleted the vpc20-service-profile2 branch August 7, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants